Skip to content

Conversation

BrennanConroy
Copy link
Member

Fixes #61084

@ghost ghost added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label Apr 4, 2025
@BrennanConroy BrennanConroy force-pushed the brecon/requestqueuesecurity branch from 10ebaa7 to cabcc2f Compare April 18, 2025 17:18
@BrennanConroy BrennanConroy marked this pull request as ready for review April 18, 2025 17:22

_disposed = true;

PInvoke.HttpCloseRequestQueue(Handle);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a reasonable change but unrelated to this PR. Drive-by bug fix or is it related somehow?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drive-by, was just reading the docs for HttpCreateRequestQueue to make sure I was doing the correct thing with the security attribute and noticed it said you should close the queue handle with HttpCloseRequestQueue.

@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Apr 25, 2025
Copy link
Member

@captainsafia captainsafia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few questions inline but LGTM overall!

HttpCancelHttpRequest
HttpCloseServerSession
HttpCloseUrlGroup
HttpCloseRequestQueue
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this file for?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's how the cswin32 source generator decides what methods to create PInvokes for
https://microsoft.github.io/CsWin32/docs/getting-started.html

@BrennanConroy BrennanConroy merged commit 4a7e556 into main Apr 28, 2025
27 checks passed
@BrennanConroy BrennanConroy deleted the brecon/requestqueuesecurity branch April 28, 2025 23:02
@BrennanConroy BrennanConroy removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Apr 28, 2025
@dotnet-policy-service dotnet-policy-service bot added this to the 10.0-preview5 milestone Apr 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions feature-httpsys

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for setting security attributes on Http.Sys RequestQueue

2 participants